-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework Muon candidate selection in few tracker ALCARECO #46574
Rework Muon candidate selection in few tracker ALCARECO #46574
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46574/42463 |
A new Pull Request was created by @mmusich for master. It involves the following packages:
@atpathak, @cmsbuild, @consuegs, @perrotta can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters:
|
type performance-improvements |
@cmsbuild, please test |
-1 Failed Tests: RelVals-INPUT
RelVals-INPUT
Expand to see more relval errors ...
Comparison SummarySummary:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
Alignment/CommonAlignmentProducer/python/TkAlMuonSelectors_cfi.py
Outdated
Show resolved
Hide resolved
…e of the selection in C++ code - replace in 4 ALCARECO producers separated configuration with a common block: TkAlGoodIdMuonSelector+TkAlRelCombIsoMuonSelector
8e49f10
to
b916211
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46574/42467 |
test parameters:
|
@cmsbuild, please test |
-1 Failed Tests: RelVals RelValsThe relvals timed out after 4 hours. |
@cmsbuild, please test let's re-try, in my local checkout area I get:
|
+1 Size: This PR adds an extra 12KB to repository Comparison SummarySummary:
|
+alca |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR is a partial response to #46493.
In #46493 (comment) it was suggested to replace few of the
MuonSelector
(ObjectSelectorBase<SingleElementCollectionSelector<edm::View<reco::Muon>, ...>
) instances with modules that express the structure of the selection in C++ code instead, as this would allow to reduce the memory footprint in Run 3 prompt reco jobs forMuon*
PDs.This is implemented in commits 8e3de65 and 2d97abd that introduce ad-hoc classes
AlignmentGoodIdMuonSelector
andAlignmentRelCombIsoMuonSelector
for this purpose.Additionally in #46493 (comment) it was suggested that the duplication of selection identical modules configuration has both memory and runtime cost. This is addressed at 8e49f10 by introducing a new common sequence
seqALCARECOTkAlRelCombIsoMuons
that ulimately takes in input areco::MuonCollection
(with labelmuons
) and returns a skimmedreco::MuonCollection
calledTkAlRelCombIsoMuonSelector
which is then used to feed the rest of the selection chain.PR validation:
runs.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
N/A